Skip to content

Conversation

@mbellade
Copy link
Member

https://hibernate.atlassian.net/browse/HHH-19880

Opening as draft, TBD:

  • module location (I picked tooling/assistant for now)
  • naming (was hibernate-tools-language)
  • Documentation ?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.


Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! A few comments below.
As long as everything is clearly marked as incubating I think it's all fine, but mostly we'll need the ack from Steve regarding the inclusion/publishing of a new module.

Comment on lines 17 to 21
- Access to data is *constrained to the mapped domain model*. The only tables the LLM will be able to access are the ones that have a corresponding entity class, and only columns listed as fields in your objects can be read. Furthermore, custom filters and SQL restrictions can be applied to further restrict the scope of the data exposed through these tools. You don’t have to worry about creating custom database-level users or permissions only to ensure sensitive information is not exposed to AI services;
- Natively maps results to *Java objects* for direct application consumption, but can also be serialized and passed back to the model to obtain an *informed natural language response based on your existing data*;
- Hibernate’s query language parsing can identify the *type of query* being executed and prevent accidental data modifications when the user only meant to read data;
- *Fail-early* in case generative AI produces incorrect statements. Thanks to Hibernate’s advanced query validation and type-safety features, we don’t need to make a round-trip to the database before noticing a problem, increasing both reliability and overall performance. It’s also easy to understand what the problem with the generated query is thanks to clear error messages, and attempt to solve it either manually or with subsequent prompts;
- With HQL it’s easier to write more *complex queries* involving multiple entities (i.e. tables) thanks to associations, embeddable values and inheritance. LLMs have an easier time generating valid queries that provide useful information to the user when compared to plain SQL, since Hibernate's query language is closer to natural language.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's fine to list features, to say what this think can help you with, and how it does so.

But I wouldn't try to make claims about safety or to compare ease of use with other solutions. That would belong in a blog post IMO.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I didn't know what to add here apart from possible use-cases / advantages of using this module, so that's what I did. If we want to keep this brief, just for reference, we can definitely remove this whole paragraph.

Copy link
Member

@yrodiere yrodiere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found two incorrect heading levels, but otherwise LGTM. Let's ask Steve on Monday?

No implementation is included, but the above provides the building blocks for integration with generative AI services/APIs.

[[assistant-gen-ai]]
==== Generative AI integration considerations
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect heading level

Suggested change
==== Generative AI integration considerations
=== Generative AI integration considerations



[[assistant-serialization]]
==== Serialization Components
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect heading level

Suggested change
==== Serialization Components
=== Serialization Components

@sebersole
Copy link
Member

module location (I picked tooling/assistant for now)

I see simply hibernate-assistant at the root level as opposed to under tooling. Was there a discussion about that that I missed? Doesn't this make more sense there?

Otherwise, looking good!

@mbellade
Copy link
Member Author

Thanks @sebersole for taking a look! My first draft put everything under tooling, but on the Jira there was discussion on what we consider to be "tooling".

I don't have a strong preference but, in its current state, the assistant module mostly contains utilities/building-blocks so I can see the argument for it to be considered a "tool".

@yrodiere
Copy link
Member

The discussion regarding the location of the module happened there: https://hibernate.atlassian.net/browse/HHH-19880?focusedCommentId=126106

No strong preference on my end either. I just thought "tools" was (up until know) about build-time tools.

@sebersole
Copy link
Member

"tools" was intended for stuff that is considered "ancillary" to the ORM runtime, but not part of it. In other words, things that a user would not load into the classpath just to use ORM.

To date, this has only been build-time tools. But I think hibernate-assistant falls under this category though, right?

At the end of the day, there is no functional difference so I am largely +0 either way; but I do think it would be nice to be consistent in where these things are placed.

@yrodiere
Copy link
Member

But I think hibernate-assistant falls under this category though, right?

Currently it is de facto in this category, as it's only used in the Quarkus Dev UI.

But that's not the (only) intent, and the end goal is to use the module more extensively, e.g. for Langchain4j integration or in MCP servers, which would put it right in the classpath of applications.

I'm personally not convinced the assistant interface itself would end up being used that way, but I certainly can see it for the tools related to serialization.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants